popover: Fix memory management of popovers

Popovers are strange in the sense that they aren't attached to a
parent directly, they rely on the relative_to widget so the toplevel
is shared, and when they have a parent, it is the toplevel itself,
not relative_to. This also means that there are conditions where the
popover loses it's parent, so they must survive unparenting.

The previous code would be floating the last reference as soon as the
parent is gone, but it was non-obvious who'd own that reference. So
fix this situation by granting the ownership of popovers to their
relative_to widget, an extra reference may be held by the toplevel
when the popover has a parent, but the popover object will be
guaranteed to be alive as long as the parent lives.

This way, memory management of popovers is as hidden from the user
as regular widgets within containers are, users are free to call
gtk_widget_destroy() on a popover, but it'd eventually become
destructed when relative_to is.

diff --git a/gtk/gtkpopover.c b/gtk/gtkpopover.c
index 55a2274da1d639d7af7bbab02aa497315502c7fb..e425641ad3041bd4040b0b1e6f72fa4254aa008a 100644 (file)
--- a/gtk/gtkpopover.c
+++ b/gtk/gtkpopover.c
@@ -70,6 +70,8 @@ struct _GtkPopoverPrivate
   guint current_position   : 2;
 };
 
+static GQuark quark_widget_popovers = 0;
+
 static void gtk_popover_update_position    (GtkPopover *popover);
 static void gtk_popover_update_relative_to (GtkPopover *popover,
                                             GtkWidget  *relative_to);
@@ -163,6 +165,7 @@ gtk_popover_dispose (GObject *object)
   if (priv->widget)
     gtk_popover_update_relative_to (popover, NULL);
 
+  gtk_widget_set_visible (GTK_WIDGET (object), FALSE);
   G_OBJECT_CLASS (gtk_popover_parent_class)->dispose (object);
 }
 
@@ -877,6 +880,8 @@ gtk_popover_class_init (GtkPopoverClass *klass)
                                                       P_("Position to place the bubble window"),
                                                       GTK_TYPE_POSITION_TYPE, GTK_POS_TOP,
                                                       GTK_PARAM_READWRITE | G_PARAM_CONSTRUCT));
+
+  quark_widget_popovers = g_quark_from_static_string ("gtk-quark-widget-popovers");
 }
 
 static void
@@ -913,12 +918,7 @@ _gtk_popover_parent_hierarchy_changed (GtkWidget  *widget,
     gtk_window_remove_popover (priv->window, GTK_WIDGET (popover));
 
   if (new_window)
-    {
-      gtk_window_add_popover (new_window, GTK_WIDGET (popover));
-      g_object_unref (popover);
-    }
-  else
-    g_object_force_floating (G_OBJECT (popover));
+    gtk_window_add_popover (new_window, GTK_WIDGET (popover));
 
   priv->window = new_window;
 
@@ -929,6 +929,8 @@ _gtk_popover_parent_hierarchy_changed (GtkWidget  *widget,
 
   if (gtk_widget_is_visible (GTK_WIDGET (popover)))
     gtk_widget_queue_resize (GTK_WIDGET (popover));
+
+  g_object_unref (popover);
 }
 
 static void
@@ -946,6 +948,40 @@ _gtk_popover_parent_size_allocate (GtkWidget     *widget,
   gtk_popover_update_position (popover);
 }
 
+static void
+widget_manage_popover (GtkWidget  *widget,
+                       GtkPopover *popover)
+{
+  GHashTable *popovers;
+
+  popovers = g_object_get_qdata (G_OBJECT (widget), quark_widget_popovers);
+
+  if (G_UNLIKELY (!popovers))
+    {
+      popovers = g_hash_table_new_full (NULL, NULL,
+                                        (GDestroyNotify) g_object_unref, NULL);
+      g_object_set_qdata_full (G_OBJECT (widget),
+                               quark_widget_popovers, popovers,
+                               (GDestroyNotify) g_hash_table_unref);
+    }
+
+  g_hash_table_add (popovers, g_object_ref_sink (popover));
+}
+
+static void
+widget_unmanage_popover (GtkWidget  *widget,
+                         GtkPopover *popover)
+{
+  GHashTable *popovers;
+
+  popovers = g_object_get_qdata (G_OBJECT (widget), quark_widget_popovers);
+
+  if (G_UNLIKELY (!popovers))
+    return;
+
+  g_hash_table_remove (popovers, popover);
+}
+
 static void
 gtk_popover_update_relative_to (GtkPopover *popover,
                                 GtkWidget  *relative_to)
@@ -957,6 +993,8 @@ gtk_popover_update_relative_to (GtkPopover *popover,
   if (priv->widget == relative_to)
     return;
 
+  g_object_ref (popover);
+
   if (priv->window)
     {
       gtk_window_remove_popover (priv->window, GTK_WIDGET (popover));
@@ -971,6 +1009,8 @@ gtk_popover_update_relative_to (GtkPopover *popover,
         g_signal_handler_disconnect (priv->widget, priv->size_allocate_id);
       if (g_signal_handler_is_connected (priv->widget, priv->unmap_id))
         g_signal_handler_disconnect (priv->widget, priv->unmap_id);
+
+      widget_unmanage_popover (priv->widget, popover);
     }
 
   priv->widget = relative_to;
@@ -993,12 +1033,16 @@ gtk_popover_update_relative_to (GtkPopover *popover,
         g_signal_connect (priv->widget, "unmap",
                           G_CALLBACK (_gtk_popover_parent_unmap),
                           popover);
+
+      /* Give ownership of the popover to widget */
+      widget_manage_popover (priv->widget, popover);
     }
 
   if (priv->window)
     gtk_window_add_popover (priv->window, GTK_WIDGET (popover));
 
   _gtk_popover_update_context_parent (popover);
+  g_object_unref (popover);
 }
 
 static void

diff --git a/gtk/gtktexthandle.c b/gtk/gtktexthandle.c
index af73f58bf729555812eec2540b7ffd041d9d2039..70de3b7a32234738a8d6d57f3ff7cdd931c47c81 100644 (file)
--- a/gtk/gtktexthandle.c
+++ b/gtk/gtktexthandle.c
@@ -237,7 +237,7 @@ _gtk_text_handle_ensure_widget (GtkTextHandle         *handle,
                         G_CALLBACK (gtk_text_handle_widget_style_updated),
                         handle);
 
-      priv->windows[pos].widget = widget;
+      priv->windows[pos].widget = g_object_ref_sink (widget);
       window = gtk_widget_get_ancestor (priv->parent, GTK_TYPE_WINDOW);
       gtk_window_add_popover (GTK_WINDOW (window), widget);
     }
@@ -305,11 +305,12 @@ gtk_text_handle_finalize (GObject *object)
 
   priv = GTK_TEXT_HANDLE (object)->priv;
 
+  /* We sank the references, unref here */
   if (priv->windows[GTK_TEXT_HANDLE_POSITION_SELECTION_START].widget)
-    gtk_widget_destroy (priv->windows[GTK_TEXT_HANDLE_POSITION_SELECTION_START].widget);
+    g_object_unref (priv->windows[GTK_TEXT_HANDLE_POSITION_SELECTION_START].widget);
 
   if (priv->windows[GTK_TEXT_HANDLE_POSITION_SELECTION_END].widget)
-    gtk_widget_destroy (priv->windows[GTK_TEXT_HANDLE_POSITION_SELECTION_END].widget);
+    g_object_unref (priv->windows[GTK_TEXT_HANDLE_POSITION_SELECTION_END].widget);
 
   G_OBJECT_CLASS (_gtk_text_handle_parent_class)->finalize (object);
 }

diff --git a/gtk/gtkwindow.c b/gtk/gtkwindow.c
index 260f57aded96b2df4fc056f83529b66fac652770..84792c7da0c831ab078d47a138dc3e8a63df4850 100644 (file)
--- a/gtk/gtkwindow.c
+++ b/gtk/gtkwindow.c
@@ -2662,12 +2662,15 @@ static void
 gtk_window_dispose (GObject *object)
 {
   GtkWindow *window = GTK_WINDOW (object);
+  GtkWindowPrivate *priv = window->priv;
 
   gtk_window_set_focus (window, NULL);
   gtk_window_set_default (window, NULL);
   unset_titlebar (window);
   remove_attach_widget (window);
 
+  g_hash_table_remove_all (priv->popovers);
+
   G_OBJECT_CLASS (gtk_window_parent_class)->dispose (object);
 }